CLOUDSTACK-4858 Honors the snapshot.backup.rightafter configuration v… #1697
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ariable Unhides snapshot.backup.rightafter from global configuration If snapshot.backup.rightafter is set to false (defaults to true), snapshots are not backed up to secondary storage
|
@jburwell @kiwiflyer @bvbharatk This is the new PR, I will post test results soon. |
|
Here are results running the following tests: component/test_snapshots.py component/test_snapshots_improvement.py component/test_snapshot_gc.py component/test_snapshot_limits.py on a KVM host using Ceph as primary and NFS as secondary: https://gist.github.com/nathanejohnson/38805b23a67d512e5051a0390ca5294c The failures look to be what you would expect, complaining mostly about snapshots not being backed up to secondary when that's precisely the behavior we're after. |
|
@nathanejohnson we making a hard push on #1692 to get all of the smoke tests to be super green. Once that PR is merged up, we will rebase and re-test this PR and there will be no more expected/"good" failures. |
|
@syed This is the PR we discussed on the call today. |
syed
approved these changes
Oct 24, 2016
| @@ -374,8 +375,24 @@ public SnapshotInfo takeSnapshot(SnapshotInfo snapshot) { | |||
|
|
|||
| snapshot = result.getSnashot(); | |||
| DataStore primaryStore = snapshot.getDataStore(); | |||
| boolean backupFlag = Boolean.parseBoolean(configDao.getValue(Config.BackupSnapshotAfterTakingSnapshot.toString())); | |||
There was a problem hiding this comment.
@nathanejohnson in #1600 I've added a flag to the create snapshot API called locationType which can take primary or secondary as the parameter. We could use that here as well. Although for this PR, a global would suffice
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-137 |
|
@blueorangutan could you send logs? I just verified that I could build a centos 7 rpm from this branch, can't reproduce the failure. Thanks! |
|
Looks like intermittent build issue, I'll re-fire builds. |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-141 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-290)
|
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-200 |
|
LGTM Tested it, works as expected! Was confused a bit, but it works fine on KVM. It seems |
|
merging this to 4.9+ branches |
|
@nathanejohnson @kiwiflyer There were some conflicts while merging to master. I went ahead and resolved as they are minor conflicts. Please verify that everything is fine on master. |
asfgit
pushed a commit
that referenced
this pull request
Feb 8, 2017
…p_rightafter CLOUDSTACK-4858 Honors the snapshot.backup.rightafter configuration variable Unhides snapshot.backup.rightafter from global configuration If snapshot.backup.rightafter is set to false (defaults to true), snapshots are not backed up to secondary storage. This is the same as PR #1644 applied to 4.9, as per @jburwell * pr/1697: CLOUDSTACK-4858 Honors the snapshot.backup.rightafter configuration variable Unhides snapshot.backup.rightafter from global configuration Signed-off-by: Rajani Karuturi <rajani.karuturi@accelerite.com>
|
@karuturi do you happen to remember which files conflicted? I seem to remember |
|
I think the conflicts were on VolumeOrchestrator.java and TemplateManagerImplTest.java |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ariable
Unhides snapshot.backup.rightafter from global configuration
If snapshot.backup.rightafter is set to false (defaults to true), snapshots are
not backed up to secondary storage.
This is the same as PR #1644 applied to 4.9, as per @jburwell